Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci-operator/step-registry/ipi/deprovision/artifacts/bootstrap: Drop gather #6336

Conversation

wking
Copy link
Member

@wking wking commented Dec 11, 2019

The installer does this automatically since openshift/installer@cad7f02a8b (openshift/installer#1822). You'd still need to SSH in to gather manually in 4.1.z, but if we think that's important we should backport the gather logic to the installer's 4.1 branch.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 11, 2019
@wking
Copy link
Member Author

wking commented Dec 11, 2019

Rehearse:

time="2019-12-11T05:45:52Z" level=info msg="registry steps changed" pr=6336 registry="map[ipi-deprovision-artifacts:0xc008c73920]"
time="2019-12-11T05:45:53Z" level=info msg="no jobs to rehearse have been found" pr=6336

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2019
@wking wking force-pushed the drop-ipi-deprovision-artifacts-bootstrap branch from 9df460d to c98027b Compare December 12, 2019 23:48
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2019
@wking wking force-pushed the drop-ipi-deprovision-artifacts-bootstrap branch from c98027b to 06448cb Compare December 12, 2019 23:49
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 12, 2019
@wking
Copy link
Member Author

wking commented Dec 12, 2019

Rebased around #6352. CC @AlexNPavel, so you can land this removal and stop have to worry about carrying dead code around ;).

@AlexNPavel
Copy link
Contributor

@wking Rehearse not choosing any jobs right now is normal, since there are no jobs using these steps that it can test. In regards to 4.1.z not having the new gather functionality, I'll ask @stevekuznetsov about it, as I'm not quite sure how that will affect us.

@wking
Copy link
Member Author

wking commented Dec 13, 2019

In regards to 4.1.z not having the new gather functionality...

Just means you don't get bootstrap logs from 4.1.z tests that fail before bootstrap removal. If that bothers folks, I think the fix is "backport the auto-gather to the installer's 4.1.z", not "write some CI hack that makes its own Terraform state assumptions".

@stevekuznetsov
Copy link
Contributor

I'd love to not regress in artifacts in case that will become an issue for people working on backports -- WDYT?

@wking
Copy link
Member Author

wking commented Dec 18, 2019

I'd love to not regress in artifacts...

I bet we end-of-life 4.1 before we hit the next issue that would need these logs for debugging. And if it does come up, I'm happy to volunteer to debug the issue if that allows me to not be on the hook to maintain this Terraform-state-poking crud ;). Also, I've floated the 4.1 backport to get the installer to gather automatically (openshift/installer#2840), although that's not my backport decision to make.

…ather

The installer does this automatically since
openshift/installer@cad7f02a8b (cmd: gather the logs from bootstrap
instead of printing commands, 2019-06-05, openshift/installer#1822).
You'd still need to SSH in to gather manually in 4.1.z, but if we
think that's important we should backport the gather logic to the
installer's 4.1 branch [1].

[1]: openshift/installer#2840
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2019
@wking wking force-pushed the drop-ipi-deprovision-artifacts-bootstrap branch from 06448cb to 970ec02 Compare December 18, 2019 10:10
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2019
@wking
Copy link
Member Author

wking commented Dec 18, 2019

Rebased again (this time around #6429) with 06448cb9c -> 970ec02. If neither this PR nor the installer backport has legs with their respective maintainers, the middle ground is probably just droppping my name from ci-operator/step-registry/ipi/deprovision/artifacts/bootstrap/OWNERS ;).

@crawford
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 8189bbb into openshift:master Jan 23, 2020
@openshift-ci-robot
Copy link
Contributor

@wking: Updated the step-registry configmap in namespace ci at cluster default using the following files:

  • key OWNERS using file ``
  • key ipi-deprovision-artifacts-bootstrap-commands.sh using file ``
  • key ipi-deprovision-artifacts-bootstrap-ref.yaml using file ``
  • key ipi-deprovision-artifacts-chain.yaml using file ci-operator/step-registry/ipi/deprovision/artifacts/ipi-deprovision-artifacts-chain.yaml

In response to this:

The installer does this automatically since openshift/installer@cad7f02a8b (openshift/installer#1822). You'd still need to SSH in to gather manually in 4.1.z, but if we think that's important we should backport the gather logic to the installer's 4.1 branch.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wking wking deleted the drop-ipi-deprovision-artifacts-bootstrap branch January 25, 2020 18:18
wking added a commit to wking/openshift-release that referenced this pull request Jan 25, 2020
…hers

Like 970ec02
(ci-operator/step-registry/ipi/deprovision/artifacts/bootstrap: Drop
gather, 2019-12-10, openshift#6336), except for the templates instead of the
step registry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants